-
Notifications
You must be signed in to change notification settings - Fork 55
chore: tweak role{group,combine}model #1152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
调整 RoleGroupModel 以及 RoleCombineModel 便于后续维护. (这是迁移自图标拆分分支的变更) Log:
|
Skipping CI for Draft Pull Request. |
deepin pr auto review关键摘要:
是否建议立即修改:
|
Reviewer's GuideRefactors RoleCombineModel by extracting and caching its roleNames generation into a dedicated method for clarity and maintenance, and adds a safety null-check in RoleGroupModel::rowCount to prevent errors when the source model is unset. Sequence Diagram: RoleCombineModel Role Name Generation, Caching, and AccesssequenceDiagram
participant ClientCode as "Client Code"
participant Constructor as "RoleCombineModel(...)"
participant RCM_Instance as "RoleCombineModel Instance"
participant CreateRN_Method as "RCM_Instance.createRoleNames()"
participant RoleNames_Method as "RCM_Instance.roleNames()"
Note over ClientCode, RoleNames_Method: Illustrates refactoring of role name generation and access in RoleCombineModel
ClientCode ->> Constructor: new RoleCombineModel(major, minor)
activate Constructor
Constructor ->> CreateRN_Method: Call to generate and combine role names
activate CreateRN_Method
Note right of CreateRN_Method: Internally fetches role names from sourceModel and m_minor
CreateRN_Method -->> Constructor: combined_role_names
deactivate CreateRN_Method
Constructor ->> RCM_Instance: m_roleNames = combined_role_names (Cache populated)
Note over Constructor, RCM_Instance: m_minorRolesMap is then populated using m_roleNames.key() (cached)
deactivate Constructor
ClientCode ->> RoleNames_Method: getRoleNames()
activate RoleNames_Method
RoleNames_Method ->> RCM_Instance: Access m_roleNames
RCM_Instance -->> RoleNames_Method: Returns cached m_roleNames
deactivate RoleNames_Method
RoleNames_Method -->> ClientCode: cached_role_names
Class Diagram: RoleCombineModel Refactoring for Role Name CachingclassDiagram
class RoleCombineModel {
+RoleCombineModel(QAbstractItemModel* major, QAbstractItemModel* minor, QObject *parent) // Logic modified for caching
+roleNames() const : QHash<int, QByteArray> // Now returns cached m_roleNames
-createRoleNames() const : QHash<int, QByteArray> // New: Encapsulates role name generation
-m_roleNames: QHash<int, QByteArray> // New: Cache for role names
-m_minor: QAbstractItemModel* // Existing relevant member
-m_minorRolesMap: QHash<int, int> // Existing relevant member, population logic changed
}
RoleCombineModel --|> QAbstractProxyModel
Class Diagram: RoleGroupModel Update to rowCount MethodclassDiagram
class RoleGroupModel {
+rowCount(const QModelIndex &parent) const : int // Logic modified: Added null check for sourceModel()
+setSourceModel(QAbstractItemModel *model) // Existing method, provides context
}
RoleGroupModel --|> QAbstractProxyModel
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @BLumia - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, yixinshark The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
调整 RoleGroupModel 以及 RoleCombineModel 便于后续维护.
(这是迁移自图标拆分分支的变更)
Log:
Summary by Sourcery
Refactor role handling in proxy models by splitting and caching roleNames in RoleCombineModel for easier maintenance, and add a safety check in RoleGroupModel::rowCount.
Enhancements: